Skip comments in Ruby files when checking for class names #19243

thecrypticace · 2025-10-31T16:42:14Z

Fixes #19239

crates/oxide/src/extractor/pre_processors/ruby.rs

coderabbitai · 2025-11-11T20:57:07Z

Walkthrough

Adds an Exclamation enum variant (b'!') and parsing logic in arbitrary_property_machine.rs to accept a top-level !important sequence as an arbitrary-property value terminator only when not bracket-nested. Enhances the Ruby pre-processor to skip single/double-quoted string contents (with backslash escapes), convert # comments to spaces until newline, and refine %w, %W, %p delimiter handling using a bracket stack plus space/newline boundary rules. Removes nonfunctional annotation lines from a HAML test fixture and adds a CHANGELOG entry about skipping Ruby comments when checking class names.

Pre-merge checks

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'Skip comments in Ruby files when checking for class names' directly aligns with the main change of improving Ruby comment handling during content scanning to reduce false parsing warnings.
Description check	✅ Passed	The PR description references issue #19239, which matches the linked issue addressing parsing warnings from Ruby RDoc syntax during content scanning.
Linked Issues check	✅ Passed	The code changes address issue #19239 by improving Ruby comment and RDoc handling: explicit comment skipping in ruby.rs, arbitrary property value termination with '!important' support, and updated tests covering these scenarios.
Out of Scope Changes check	✅ Passed	Changes to arbitrary_property_machine.rs for '!important' handling are closely related to resolving the RDoc parsing issue by enabling proper CSS value termination, while haml fixture changes are minimal cleanup.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f381118 and 7f34f3f.

📒 Files selected for processing (1)

crates/oxide/src/extractor/pre_processors/ruby.rs (5 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

crates/oxide/src/extractor/pre_processors/ruby.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)

GitHub Check: Linux
GitHub Check: Linux / oxide
GitHub Check: Linux / webpack
GitHub Check: Linux / upgrade
GitHub Check: Linux / cli
GitHub Check: Linux / vite
GitHub Check: Linux / postcss

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3cece1d and c19d255.

📒 Files selected for processing (3)

crates/oxide/src/extractor/arbitrary_property_machine.rs (3 hunks)
crates/oxide/src/extractor/pre_processors/ruby.rs (5 hunks)
crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml (0 hunks)

💤 Files with no reviewable changes (1)

crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml

🧰 Additional context used

🧬 Code graph analysis (1)

crates/oxide/src/extractor/pre_processors/ruby.rs (2)

crates/oxide/src/extractor/candidate_machine.rs (1)

next (29-169)

crates/oxide/src/extractor/string_machine.rs (1)

next (32-68)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

GitHub Check: Linux
GitHub Check: Linux / postcss
GitHub Check: Linux / vite
GitHub Check: Linux / upgrade

crates/oxide/src/extractor/arbitrary_property_machine.rs

RobinMalfait

Looks good, but not sure about the !important.

Can you also add a changelog entry?

crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml

crates/oxide/src/extractor/arbitrary_property_machine.rs

thecrypticace · 2025-11-12T15:41:09Z

Yeah I was not gonna change that part. It's unrelated anyway. Will get the change log updated.

Unless it’s part of `!important` at the end

coderabbitai

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c19d255 and f381118.

📒 Files selected for processing (4)

CHANGELOG.md (1 hunks)
crates/oxide/src/extractor/arbitrary_property_machine.rs (3 hunks)
crates/oxide/src/extractor/pre_processors/ruby.rs (5 hunks)
crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml (0 hunks)

💤 Files with no reviewable changes (1)

crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml

🧰 Additional context used

🧬 Code graph analysis (1)

crates/oxide/src/extractor/pre_processors/ruby.rs (2)

crates/oxide/src/extractor/candidate_machine.rs (1)

next (29-169)

crates/oxide/src/extractor/string_machine.rs (1)

next (32-68)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

GitHub Check: Linux
GitHub Check: Linux / cli
GitHub Check: Linux / upgrade
GitHub Check: Linux / postcss
GitHub Check: Linux / vite
GitHub Check: Linux / webpack

coderabbitai · 2025-11-12T17:20:04Z

crates/oxide/src/extractor/pre_processors/ruby.rs

+                // Replace comments in Ruby files
+                b'#' => {
+                    result[cursor.pos] = b' ';
+                    cursor.advance();
+
+                    while cursor.pos < len {
+                        match cursor.curr {
+                            // End of the comment
+                            b'\n' => break,
+
+                            // Everything else is part of the comment and replaced
+                            _ => {
+                                result[cursor.pos] = b' ';
+                                cursor.advance();
+                            }
+                        };
+                    }
+
+                    cursor.advance();
+                    continue;
+                }
+


⚠️ Potential issue | 🟠 Major

Avoid clobbering percent literals when stripping comments.

Treating every # outside of quoted strings as the start of a comment nukes valid Ruby percent literals like %r#foo#bar# or %i#klass#, because this loop replaces everything from the delimiter onward with spaces. After preprocessing, the literal body is gone and the extractor can’t see classes tucked inside. Please keep %r/%i/%s-style delimiters (and similar % forms) intact—e.g., bail out of the comment branch when the # immediately follows a %[a-zA-Z] sequence—so we only blank true comments.

🤖 Prompt for AI Agents

In crates/oxide/src/extractor/pre_processors/ruby.rs around lines 121 to 142, the comment-stripping branch treats every '#' as the start of a comment and thus clobbers Ruby percent-literals like %r#...# or %i#...#; change the branch to detect when the '#' is actually the delimiter of a percent-literal and skip comment removal in that case. Concretely, before turning the '#' into a space and entering the comment loop, check the two bytes immediately before cursor (or the original buffer at cursor.pos-2 and cursor.pos-1) and if they form '%' followed by an ASCII letter (/[A-Za-z]/) then do not treat this '#' as a comment (bail out of the comment branch and continue normal scanning), otherwise proceed with the existing comment-blanking logic.

thecrypticace force-pushed the fix/issue-19239 branch from bc1c637 to fa2be75 Compare October 31, 2025 16:42

thecrypticace marked this pull request as ready for review October 31, 2025 18:21

thecrypticace requested a review from a team as a code owner October 31, 2025 18:21

RobinMalfait reviewed Nov 3, 2025

View reviewed changes

crates/oxide/src/extractor/pre_processors/ruby.rs Show resolved Hide resolved

coderabbitai bot reviewed Nov 11, 2025

View reviewed changes

crates/oxide/src/extractor/arbitrary_property_machine.rs Show resolved Hide resolved

RobinMalfait requested changes Nov 12, 2025

View reviewed changes

crates/oxide/src/extractor/pre_processors/test-fixtures/haml/dst-17051.haml Show resolved Hide resolved

crates/oxide/src/extractor/arbitrary_property_machine.rs Show resolved Hide resolved

thecrypticace added 9 commits November 12, 2025 12:14

Skip over strings in Ruby files

5c54516

Replace comments in Ruby code

54617bc

Discard arbitrary properties with ! at the top-level

a413e53

Unless it’s part of `!important` at the end

Fix formatting

956ce18

Add tests

d9163d8

Support single line string literal specifiers

3778c28

Only match expected string literal specifiers

14f888f

Fix formatting

5662aaf

Update changelog

f381118

thecrypticace force-pushed the fix/issue-19239 branch from 42f48be to f381118 Compare November 12, 2025 17:15

coderabbitai bot reviewed Nov 12, 2025

View reviewed changes

Handle # as a string format delimiter

7f34f3f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Skip comments in Ruby files when checking for class names #19243

Skip comments in Ruby files when checking for class names #19243

thecrypticace commented Oct 31, 2025

Uh oh!

Uh oh!

coderabbitai bot commented Nov 11, 2025 •

edited

Loading

Uh oh!

coderabbitai bot left a comment

Uh oh!

Uh oh!

RobinMalfait left a comment

Uh oh!

Uh oh!

Uh oh!

thecrypticace commented Nov 12, 2025

Uh oh!

coderabbitai bot left a comment

Uh oh!

coderabbitai bot Nov 12, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Skip comments in Ruby files when checking for class names #19243

Are you sure you want to change the base?

Skip comments in Ruby files when checking for class names #19243

Conversation

thecrypticace commented Oct 31, 2025

Uh oh!

Uh oh!

coderabbitai bot commented Nov 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Pre-merge checks

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

RobinMalfait left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

thecrypticace commented Nov 12, 2025

Uh oh!

coderabbitai bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai bot Nov 12, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

coderabbitai bot commented Nov 11, 2025 •

edited

Loading